coordinator/test/api_test.go (1)

135-135: Ensure version bump intent and keep test expectations in sync

Setting version.Version = "v4.5.45" is fine for simulating a newer prover; just confirm it aligns with the configured min_prover_version in these tests (currently "v4.4.89" in setupCoordinator) and with runtime defaults to avoid drift.

Optionally, define a single test constant for the minimum version and reuse it in both the conf and expected error strings to avoid future mismatches.

coordinator/conf/config.json (1)

25-25: Avoid committing literal DB credentials in templates

Hardcoding dev:dev triggers secret scanners and can leak into ephemeral environments.

Apply this diff to use a neutral placeholder (compatible with local envsubst or manual edits):

-    "dsn": "postgres://dev:dev@localhost/scroll?sslmode=disable",
+    "dsn": "postgres://${DB_USER}:${DB_PASS}@${DB_HOST:-localhost}/scroll?sslmode=disable",

If you don’t plan to expand env vars, at least switch to obvious placeholders to silence scanners (e.g., :).

Also confirm min_prover_version here matches your rollout plan (PR text mentions compatibility with 4.5.33; current value is "v4.4.45").

coordinator/internal/logic/libzkp/libzkp.h (1)

59-60: Clarify ownership and use const for the input parameter

Match the Rust C-ABI (takes *const c_char) and document that the caller must free the returned string via release_string to avoid leaks.

Apply this diff:

-// Universal task compatibility fix function
-char* univ_task_compatibility_fix(char* task_json);
+// Universal task compatibility fix function
+// Returns a newly-allocated JSON string; caller must call release_string() on the result.
+char* univ_task_compatibility_fix(const char* task_json);

crates/libzkp_c/src/lib.rs (1)

241-252: Fix typo in error log and add a null check for robustness.

• Typo: "compability" → "compatibility".
• Add a null check to avoid UB if a non-Go caller ever passes NULL.

 pub unsafe extern "C" fn univ_task_compatibility_fix(task_json: *const c_char) -> *mut c_char {
-    let task_json_str = c_char_to_str(task_json);
+    if task_json.is_null() {
+        tracing::error!("univ_task_compatibility_fix received null task_json");
+        return std::ptr::null_mut();
+    }
+    let task_json_str = c_char_to_str(task_json);
     match libzkp::univ_task_compatibility_fix(task_json_str) {
         Ok(result) => CString::new(result).unwrap().into_raw(),
         Err(e) => {
-            tracing::error!("univ_task_compability_fix failed, error: {:#}", e);
+            tracing::error!("univ_task_compatibility_fix failed, error: {:#}", e);
             std::ptr::null_mut()
         }
     }
 }

coordinator/build/setup_releases.sh (1)

10-10: Harden the script: strict mode, tool checks, and download failure handling.

Add strict flags and pre-flight checks; consider retrying downloads to avoid flaky CI.

Additional changes (outside the selected lines) to add after the shebang:

set -Eeuo pipefail

command -v jq >/dev/null || { echo "jq is required"; exit 1; }
command -v wget >/dev/null || { echo "wget is required"; exit 1; }

Optionally wrap wget with retries:

wget --tries=3 --timeout=30 https://circuit-release.s3.us-west-2.amazonaws.com/scroll-zkvm/releases/$SCROLL_ZKVM_VERSION/verifier/verifier.bin -O "${ASSET_DIR}/verifier.bin"
wget --tries=3 --timeout=30 https://circuit-release.s3.us-west-2.amazonaws.com/scroll-zkvm/releases/$SCROLL_ZKVM_VERSION/verifier/root_verifier_vk -O "${ASSET_DIR}/root_verifier_vk"
wget --tries=3 --timeout=30 https://circuit-release.s3.us-west-2.amazonaws.com/scroll-zkvm/releases/$SCROLL_ZKVM_VERSION/verifier/openVmVk.json -O "${ASSET_DIR}/openVmVk.json"

Also applies to: 56-56

tests/prover-e2e/Makefile (1)

4-5: Validate numeric inputs for BEGIN_BLOCK/END_BLOCK.

Prevent accidental non-numeric values that could break the tool invocation.

 check_vars:
-	@if [ -z "$(BEGIN_BLOCK)" ] || [ -z "$(END_BLOCK)" ]; then \
+	@if [ -z "$(BEGIN_BLOCK)" ] || [ -z "$(END_BLOCK)" ]; then \
         echo "Error: BEGIN_BLOCK and END_BLOCK must be defined"; \
         echo "Usage: make import_data BEGIN_BLOCK=<start_block> END_BLOCK=<end_block>"; \
         exit 1; \
     fi
+	@if ! echo "$(BEGIN_BLOCK)" | grep -Eq '^[0-9]+$$'; then \
+        echo "Error: BEGIN_BLOCK must be numeric"; exit 1; \
+    fi
+	@if ! echo "$(END_BLOCK)" | grep -Eq '^[0-9]+$$'; then \
+        echo "Error: END_BLOCK must be numeric"; exit 1; \
+    fi

Cargo.toml (1)

20-23: Nit: fix comment typo and consider aligning workspace version with release.

• Typo: "compatiblity" → "compatibility".
• Optional: bump [workspace.package].version from 4.5.47 to 4.6.1 for consistency with the PR headline.

-# include compatiblity fixing from "fix/coordinator"
+# include compatibility fixing from "fix/coordinator"

If intended, also update (outside this hunk):

• version = "4.5.47" -> "4.6.1"

crates/libzkp/src/lib.rs (1)

49-88: Polish docs; verify byte encoding expectations for compatibility.

• Fix typos in doc comments.
• Confirm whether vk should serialize as base64 (like elsewhere via vec_as_base64). If the old prover expects raw arrays, current code is fine; otherwise, add serde attribute.

 /// Convert the universal task json into compatible form for old prover
 pub fn univ_task_compatibility_fix(task_json: &str) -> eyre::Result<String> {
@@
     #[derive(Serialize)]
     struct CompatibleProvingTask {
-        /// seralized witness which should be written into stdin first
+        /// serialized witness which should be written into stdin first
         pub serialized_witness: Vec<Vec<u8>>,
         /// aggregated proof carried by babybear fields, should be written into stdin
-        /// followed `serialized_witness`
+        /// followed by `serialized_witness`
         pub aggregated_proofs: Vec<VmInternalStarkProof>,
         /// Fork name specify
         pub fork_name: String,
-        /// The vk of app which is expcted to prove this task
+        /// The vk of the app which is expected to prove this task
         pub vk: Vec<u8>,
-        /// An identifier assigned by coordinator, it should be kept identify for the
-        /// same task (for example, using chunk, batch and bundle hashes)
+        /// An identifier assigned by coordinator; it should be kept identical for the
+        /// same task (e.g., using chunk, batch, and bundle hashes)
         pub identifier: String,
     }

If base64 encoding is required for vk, use:

• Add use scroll_zkvm_types::utils::vec_as_base64; (already available in this module)
• Annotate: #[serde(with = "vec_as_base64")] pub vk: Vec<u8>,

coordinator/internal/logic/provertask/prover_task.go (2)

207-210: Function naming and minor robustness

Rename to reads-as-boolean and trim input; use the renamed constant.

Apply this diff:

-func isCompatibilityFixingVersion(ver string) bool {
-	return !version.CheckScrollRepoVersion(ver, CompatibilityVersion)
-}
+// requiresCompatibilityFix returns true if the prover version is below MinCompatFixVersion.
+func requiresCompatibilityFix(ver string) bool {
+	ver = strings.TrimSpace(ver)
+	return !version.CheckScrollRepoVersion(ver, MinCompatFixVersion)
+}

Note: Update call sites accordingly in batch_prover_task.go and bundle_prover_task.go.

---

211-220: Defensive nil-check and error context in fixCompatibility

Avoid potential panic on nil schema and wrap errors for easier triage. Also confirm that only TaskData needs adjustment (no HardForkName/UseSnark changes for older provers).

Apply this diff:

-func fixCompatibility(schema *coordinatorType.GetTaskSchema) error {
-
-	fixedTask, err := libzkp.UnivTaskCompatibilityFix(schema.TaskData)
-	if err != nil {
-		return err
-	}
-	schema.TaskData = fixedTask
-
-	return nil
-}
+// fixCompatibility applies the universal-task compatibility fix in-place.
+func fixCompatibility(schema *coordinatorType.GetTaskSchema) error {
+	if schema == nil {
+		return errors.New("fixCompatibility: nil schema")
+	}
+	fixedTask, err := libzkp.UnivTaskCompatibilityFix(schema.TaskData)
+	if err != nil {
+		return fmt.Errorf("fixCompatibility: UnivTaskCompatibilityFix failed: %w", err)
+	}
+	schema.TaskData = fixedTask
+	return nil
+}

If the legacy task format also differed in fields other than TaskData for < MinCompatFixVersion, adjust here (or document why only TaskData is impacted).

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro